lib: change abstract equal to strict equal#22974
lib: change abstract equal to strict equal#22974ZYSzys wants to merge 2 commits intonodejs:masterfrom zys-contrib:strict
Conversation
nodejs-github-bot
added
the
console
| throw new ERR_INVALID_ARG_TYPE('properties', 'Array', properties); | ||
|
|
||
| if (tabularData == null || typeof tabularData !== 'object') | ||
| if (tabularData === null || typeof tabularData !== 'object') |
There was a problem hiding this comment.
It's not obvious, but this significantly changes the semantics of the check in a non-backwards compatible way.
null == null; // true
undefined == null; // true
null === null; // true
undefined === null; // falseThe current check makes sure that tabularData is also not undefined.
There was a problem hiding this comment.
But the later one can check it, typeof undefined === 'undefined' so typeof tabularData !== 'object' is true.
There was a problem hiding this comment.
This is 100 % subjective, but maybe this would be a bit more Obviously Correct™ if the conditions were switched?
There was a problem hiding this comment.
Just not seeing the point of the change then if it simply shifts the undefined case a bit to the right.
There was a problem hiding this comment.
Just change to strict equal to unify the overall code style.
If there is any problem, feel free to close it.😜
There was a problem hiding this comment.
Nah, I'm not overly happy with it but I won't block it :-)
addaleax
added
the
author ready
|
Resumed build: https://ci.nodejs.org/job/node-test-pull-request/17424/ |
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
|
Thanks all of you. |
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: #22974 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
8 participants
Just change the rare
==to===, nothing else be affected.Unify the style, may looks more comfortable 😄.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes